-
Notifications
You must be signed in to change notification settings - Fork 408
feat(clerk-js,clerk-react,vue): Introduce development modal to enable organizations #7159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 6f4897e The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds an in-app development prompt and internal APIs to enable the Organizations feature from the app; introduces a DevTools endpoint, new UI components/providers/hooks, SSR premount plumbing, type and appearance additions, lazy-module wiring, and replaces scattered org-flag checks with a centralized enablement flow. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Org Component
participant Clerk as Clerk Core
participant Modal as EnableOrgsPrompt
participant DevTools as DevTools API
Note over Clerk,Modal: Centralized enablement flow
Component->>Clerk: __internal_attemptToEnableEnvironmentSetting({for: "organizations", caller})
alt development & not enabled
Clerk->>Modal: __internal_openEnableOrganizationsPrompt(props)
Modal->>Developer: render prompt (toggle, enable)
Developer->>Modal: click Enable
Modal->>DevTools: __internal_enableEnvironmentSetting(params)
DevTools-->>Modal: success
Modal->>Browser: trigger reload / call onSuccess
Clerk-->>Component: { status: "prompt-shown" }
else enabled or production
Clerk-->>Component: { status: "enabled" }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35–45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
Comment |
2014336 to
c9c9e99
Compare
c9c9e99 to
b8fb3db
Compare
b8fb3db to
06a951f
Compare
06a951f to
c990467
Compare
c990467 to
9ade3ae
Compare
8953897 to
a3f9bed
Compare
a3f9bed to
c145bf9
Compare
c145bf9 to
b456c24
Compare
b456c24 to
81ce300
Compare
81ce300 to
b2b3ad4
Compare
a34a6ce to
60e0b72
Compare
|
Hey @LauraBeatris - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.2.4-snapshot.v20251121134419 --save-exact
npm i @clerk/astro@2.16.2-snapshot.v20251121134419 --save-exact
npm i @clerk/backend@2.23.2-snapshot.v20251121134419 --save-exact
npm i @clerk/chrome-extension@2.8.4-snapshot.v20251121134419 --save-exact
npm i @clerk/clerk-js@5.110.0-snapshot.v20251121134419 --save-exact
npm i @clerk/elements@0.23.85-snapshot.v20251121134419 --save-exact
npm i @clerk/clerk-expo@2.19.4-snapshot.v20251121134419 --save-exact
npm i @clerk/expo-passkeys@0.4.21-snapshot.v20251121134419 --save-exact
npm i @clerk/express@1.7.52-snapshot.v20251121134419 --save-exact
npm i @clerk/fastify@2.6.4-snapshot.v20251121134419 --save-exact
npm i @clerk/localizations@3.28.3-snapshot.v20251121134419 --save-exact
npm i @clerk/nextjs@6.35.4-snapshot.v20251121134419 --save-exact
npm i @clerk/nuxt@1.13.2-snapshot.v20251121134419 --save-exact
npm i @clerk/clerk-react@5.57.0-snapshot.v20251121134419 --save-exact
npm i @clerk/react-router@2.2.4-snapshot.v20251121134419 --save-exact
npm i @clerk/remix@4.13.19-snapshot.v20251121134419 --save-exact
npm i @clerk/shared@3.36.0-snapshot.v20251121134419 --save-exact
npm i @clerk/tanstack-react-start@0.27.4-snapshot.v20251121134419 --save-exact
npm i @clerk/testing@1.13.18-snapshot.v20251121134419 --save-exact
npm i @clerk/themes@2.4.39-snapshot.v20251121134419 --save-exact
npm i @clerk/types@4.101.2-snapshot.v20251121134419 --save-exact
npm i @clerk/vue@1.17.0-snapshot.v20251121134419 --save-exact |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (6)
packages/clerk-js/src/core/clerk.ts (2)
788-800: UseeventPrebuiltComponentOpenedfor prompt telemetry to match other modals
__internal_openEnableOrganizationsPromptcurrently recordseventPrebuiltComponentMounted('EnableOrganizationsPrompt', props), while other “open modal” methods (openSignIn,openUserProfile, etc.) useeventPrebuiltComponentOpened. For consistency and clearer analytics semantics (“prompt shown” vs “component mounted”), consider:- this.telemetry?.record(eventPrebuiltComponentMounted('EnableOrganizationsPrompt', props)); + this.telemetry?.record(eventPrebuiltComponentOpened('EnableOrganizationsPrompt', props));This keeps telemetry for the new prompt aligned with existing prebuilt components.
750-785: Avoid unsafe cast and align prompt propscallerunion with attempt params
__internal_attemptToEnableEnvironmentSettingstill needs to coerce the object passed to__internal_openEnableOrganizationsPromptviaas __internal_EnableOrganizationsPromptProps, becausecallerin__internal_AttemptToEnableEnvironmentSettingParamsincludes'CreateOrganization'and'TaskChooseOrganization'while__internal_EnableOrganizationsPromptProps['caller']does not.To keep types honest and avoid this cast:
- Extend
__internal_EnableOrganizationsPromptProps['caller']to include all values from__internal_AttemptToEnableEnvironmentSettingParams['caller'](or extract a shared union), then- Drop the
as __internal_EnableOrganizationsPromptPropsassertion and pass the object directly.This restores exhaustiveness and prevents subtle bugs if the prompt later branches on new
callervalues.packages/react/src/isomorphicClerk.ts (1)
1490-1499: Clarify semantics of__internal_attemptToEnableEnvironmentSettingwhen ClerkJS isn’t loadedRight now,
__internal_attemptToEnableEnvironmentSettingqueues a premount callback and returnsvoidwhenclerkjsis not yet loaded. Callers must therefore defensively handle anundefinedresult, even though the underlying core method always returns a status.Given this API is used to gate rendering and decide whether to show the dev prompt, consider instead:
- Returning an explicit rejected state when ClerkJS is unavailable (e.g.
{ status: 'rejected' }), or- Clearly documenting and typing the
voidcase at call sites so they don’t assume a status is always present.This avoids silent no‑ops in SSR/early‑mount scenarios where enablement decisions are needed synchronously.
packages/shared/src/types/clerk.ts (1)
323-339: Align caller unions and documentrejectedstatus to keep internal types coherentA couple of type‑surface inconsistencies remain around the new enable‑organizations flow:
Caller unions are still out of sync
__internal_EnableOrganizationsPromptProps['caller']only allows:
'OrganizationSwitcher' | 'OrganizationProfile' | 'OrganizationList' | 'useOrganizationList' | 'useOrganization'.__internal_AttemptToEnableEnvironmentSettingParams['caller']additionally allows:
'CreateOrganization' | 'TaskChooseOrganization'.This mismatch is what forces the cast in
Clerk.__internal_attemptToEnableEnvironmentSetting. Recommend extracting a sharedtype __internal_OrganizationCaller = …and using it in both definitions so all entrypoints are covered and the cast can be removed. (Also see the corresponding call site inpackages/clerk-js/src/core/clerk.ts.)
rejectedstatus isn’t described or currently produced
__internal_AttemptToEnableEnvironmentSettingResultincludes a'rejected'status, but the Clerk implementation currently only returns'enabled'or'prompt-shown', and the docblock for__internal_attemptToEnableEnvironmentSettingdoesn’t mention'rejected'.Either:
- Start returning
'rejected'from appropriate callers (e.g. when enablement can’t be attempted), or- Remove
'rejected'from the union and adjust the doc comment accordingly.Keeping these internal types and docs tightly aligned will make this enablement flow easier to evolve safely.
Also applies to: 1449-1476
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (2)
54-71: Includewindow.location.hrefin useMemo dependencies.The
useMemohook at line 56 readswindow.location.hrefbut doesn't list it in the dependency array (line 71). Whilewindow.location.hrefrarely changes in SPAs, including it ensures the memo recomputes correctly if the URL does change.Apply this diff:
const organizationsDashboardUrl = useMemo(() => { return withLastActiveFallback(() => { const currentUrl = window.location.href; try { const redirectUrlParts = handleDashboardUrlParsing(currentUrl); const url = new URL( `${redirectUrlParts.baseDomain}/apps/${redirectUrlParts.appId}/instances/${redirectUrlParts.instanceId}/organizations`, ); return url.href; } catch { if (!environment?.id) { throw new Error('Cannot construct dashboard URL'); } return 'https://dashboard.clerk.com/last-active?path=organization-settings'; } }); - }, [environment?.id]); + }, [environment?.id, window.location.href]);
658-681: Add hover and focus-visible styles to Link component.The
Linkcomponent lacks hover and focus-visible styles, which affects usability and accessibility. Interactive elements should provide visual feedback on hover and keyboard focus.Apply this diff:
const Link = forwardRef<HTMLAnchorElement, React.ComponentProps<'a'> & { css?: SerializedStyles }>( ({ children, css: cssProp, ...props }, ref) => { return ( <a ref={ref} {...props} css={[ basePromptElementStyles, css` color: #a8a8ff; font-size: inherit; font-weight: 500; line-height: 1.3; font-size: 0.8125rem; min-width: 0; + cursor: pointer; + transition: color 120ms ease-in-out; + + &:hover { + color: #c4c4ff; + } + + &:focus-visible { + outline: 2px solid white; + outline-offset: 2px; + border-radius: 2px; + } + + @media (prefers-reduced-motion: reduce) { + transition: none; + } `, cssProp, ]} > {children} </a> ); }, );
🧹 Nitpick comments (2)
packages/shared/src/react/hooks/useOrganization.tsx (1)
283-283: LGTM: Hook invocation is well-integrated.The
useAttemptToEnableOrganizationshook is correctly placed after the provider assertion and before organization-related logic. The implementation properly guards against duplicate attempts and handles cases where the enablement API isn't available.Optional: Consider adding JSDoc documentation.
While the PR objectives note that JSDoc is incomplete, adding a brief comment explaining that this hook triggers the development-mode organizations enablement flow would improve maintainability for future developers.
Example:
useAssertWrappedByClerkProvider('useOrganization'); +// Attempt to enable organizations in development mode if not already enabled useAttemptToEnableOrganizations('useOrganization');packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (1)
73-93: Consider adding user feedback for enablement failures.The catch block (lines 90-92) resets
isLoadingbut doesn't provide visual feedback about what went wrong. Users see the button become enabled again without knowing if the request failed or why.Consider adding an error state and displaying it to users:
const [isLoading, setIsLoading] = useState(false); const [isEnabled, setIsEnabled] = useState(false); +const [error, setError] = useState<string | null>(null); const handleEnableOrganizations = () => { setIsLoading(true); + setError(null); const params: EnableEnvironmentSettingParams = { enable_organizations: true, }; if (hasPersonalAccountsEnabled) { params.organization_allow_personal_accounts = allowPersonalAccount; } void new DevTools() .__internal_enableEnvironmentSetting(params) .then(() => { setIsEnabled(true); setIsLoading(false); }) .catch(() => { setIsLoading(false); + setError('Failed to enable Organizations. Please try again or contact support.'); }); };Then display
errorin the UI below the description text when it's set.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (28)
.changeset/flat-ravens-call.md(1 hunks)packages/clerk-js/bundlewatch.config.json(2 hunks)packages/clerk-js/src/core/clerk.ts(9 hunks)packages/clerk-js/src/core/resources/DevTools.ts(1 hunks)packages/clerk-js/src/ui/Components.tsx(11 hunks)packages/clerk-js/src/ui/components/APIKeys/APIKeys.tsx(2 hunks)packages/clerk-js/src/ui/components/KeylessPrompt/ClerkLogoIcon.tsx(0 hunks)packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx(1 hunks)packages/clerk-js/src/ui/components/devPrompts/KeylessPrompt/index.tsx(9 hunks)packages/clerk-js/src/ui/components/devPrompts/KeylessPrompt/use-revalidate-environment.ts(1 hunks)packages/clerk-js/src/ui/components/devPrompts/shared.tsx(1 hunks)packages/clerk-js/src/ui/customizables/parseAppearance.ts(1 hunks)packages/clerk-js/src/ui/elements/Modal.tsx(2 hunks)packages/clerk-js/src/ui/elements/contexts/index.tsx(1 hunks)packages/clerk-js/src/ui/lazyModules/components.ts(4 hunks)packages/clerk-js/src/ui/lazyModules/providers.tsx(1 hunks)packages/react/src/isomorphicClerk.ts(5 hunks)packages/shared/src/organization.ts(2 hunks)packages/shared/src/react/__tests__/commerce.test.tsx(1 hunks)packages/shared/src/react/commerce.tsx(2 hunks)packages/shared/src/react/hooks/useCheckout.ts(3 hunks)packages/shared/src/react/hooks/useOrganization.tsx(2 hunks)packages/shared/src/react/hooks/useOrganizationList.tsx(2 hunks)packages/shared/src/types/appearance.ts(2 hunks)packages/shared/src/types/clerk.ts(2 hunks)packages/shared/src/types/devtools.ts(1 hunks)packages/shared/src/types/index.ts(1 hunks)packages/vue/src/composables/useOrganization.ts(2 hunks)
💤 Files with no reviewable changes (1)
- packages/clerk-js/src/ui/components/KeylessPrompt/ClerkLogoIcon.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/shared/src/react/commerce.tsx
- packages/shared/src/react/hooks/useOrganizationList.tsx
- packages/clerk-js/src/ui/components/devPrompts/KeylessPrompt/use-revalidate-environment.ts
- packages/clerk-js/src/core/resources/DevTools.ts
- .changeset/flat-ravens-call.md
- packages/clerk-js/src/ui/components/devPrompts/shared.tsx
- packages/shared/src/types/devtools.ts
- packages/shared/src/organization.ts
- packages/clerk-js/src/ui/elements/contexts/index.tsx
- packages/shared/src/react/hooks/useCheckout.ts
- packages/vue/src/composables/useOrganization.ts
🧰 Additional context used
🧬 Code graph analysis (9)
packages/clerk-js/src/ui/elements/Modal.tsx (2)
packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
withFloatingTree(140-155)packages/clerk-js/src/ui/hooks/useScrollLock.ts (1)
useScrollLock(69-85)
packages/clerk-js/src/ui/components/devPrompts/KeylessPrompt/index.tsx (1)
packages/clerk-js/src/ui/components/devPrompts/shared.tsx (3)
basePromptElementStyles(33-55)PromptContainer(10-28)PromptSuccessIcon(57-82)
packages/clerk-js/src/ui/lazyModules/components.ts (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (1)
EnableOrganizationsPrompt(433-439)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (5)
packages/shared/src/types/clerk.ts (1)
__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/ui/components/devPrompts/shared.tsx (5)
handleDashboardUrlParsing(84-100)PromptContainer(10-28)PromptSuccessIcon(57-82)ClerkLogoIcon(105-176)basePromptElementStyles(33-55)packages/shared/src/types/devtools.ts (1)
EnableEnvironmentSettingParams(3-6)packages/clerk-js/src/core/resources/DevTools.ts (1)
DevTools(8-21)packages/clerk-js/src/ui/elements/Modal.tsx (1)
Modal(26-111)
packages/shared/src/react/hooks/useOrganization.tsx (1)
packages/shared/src/organization.ts (1)
useAttemptToEnableOrganizations(27-43)
packages/react/src/isomorphicClerk.ts (1)
packages/shared/src/types/clerk.ts (3)
__internal_EnableOrganizationsPromptProps(1449-1459)__internal_AttemptToEnableEnvironmentSettingParams(1461-1472)__internal_AttemptToEnableEnvironmentSettingResult(1474-1476)
packages/clerk-js/src/core/clerk.ts (3)
packages/shared/src/types/clerk.ts (2)
__internal_AttemptToEnableEnvironmentSettingParams(1461-1472)__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/utils/componentGuards.ts (1)
disabledOrganizationsFeature(21-23)packages/shared/src/telemetry/events/component-mounted.ts (1)
eventPrebuiltComponentMounted(69-75)
packages/clerk-js/src/ui/lazyModules/providers.tsx (3)
packages/shared/src/types/appearance.ts (1)
Appearance(1057-1131)packages/clerk-js/src/ui/router/VirtualRouter.tsx (1)
VirtualRouter(15-67)packages/clerk-js/src/ui/customizables/index.ts (1)
AppearanceProvider(9-9)
packages/clerk-js/src/ui/Components.tsx (4)
packages/shared/src/types/clerk.ts (1)
__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/ui/lazyModules/providers.tsx (1)
LazyEnableOrganizationsPromptProvider(212-227)packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (1)
EnableOrganizationsPrompt(433-439)packages/clerk-js/src/ui/lazyModules/components.ts (1)
EnableOrganizationsPrompt(49-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, shared, clerk-js, RQ)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (23)
packages/shared/src/react/hooks/useOrganization.tsx (1)
1-1: LGTM: Import addition is correct.The import of
useAttemptToEnableOrganizationsis properly added alongside the existinggetCurrentOrganizationMembershipimport.packages/shared/src/react/__tests__/commerce.test.tsx (1)
56-64: Vitest partial mock of../contextslooks correctUsing
vi.importActualand overriding onlyuseOrganizationContextis a good way to forceorganization: nullwhile preserving other exports; this keeps the test aligned with the new context-based API without affecting unrelated behavior.packages/shared/src/types/index.ts (1)
15-15: Re‑exportingdevtoolstypes is consistent with existing barrel patternThe additional
export type * from './devtools';cleanly exposes the new devtools types alongside the rest of the public surface.packages/clerk-js/src/ui/elements/Modal.tsx (1)
14-24: ForwardinginitialFocusReftoPopoveris a solid accessibility improvementThreading
initialFocusRefthroughModalPropsand intoPopover.initialFocuslets callers control initial focus without changing existing behavior; the types line up with the commonnumber | refpattern.Also applies to: 26-36, 55-63
packages/clerk-js/src/core/clerk.ts (1)
871-885: Centralized enablement flow for organization components looks coherentRouting all organization-related open/mount entrypoints through
__internal_attemptToEnableEnvironmentSettingand short‑circuiting onstatus === 'prompt-shown'gives a single dev‑only gate for disabled organizations, while preserving non‑dev behavior. The per‑calleronClosecallbacks that throw a targetedClerkRuntimeErrorin dev also match existing error‑code patterns.Also applies to: 908-922, 1057-1071, 1104-1118, 1142-1156, 1188-1202, 1393-1403
packages/react/src/isomorphicClerk.ts (1)
121-138: Premount/open behavior forEnableOrganizationsPromptis consistent with other modalsStoring props in
preopenEnableOrganizationsPromptwhen ClerkJS isn’t loaded yet, replaying them inhydrateClerkJS, and clearing them in__internal_closeEnableOrganizationsPromptmirrors the existing preopen patterns (SignIn, Checkout, etc.). The truthy check in hydration is fine since the field is initialized tonulland only ever set to an object ornull.Also applies to: 563-567, 587-629, 631-633, 876-890
packages/clerk-js/src/ui/components/APIKeys/APIKeys.tsx (1)
2-7: Switching touseOrganizationContextis a good way to sidestep the dev prompt for API keysDeriving
subjectfromuseOrganizationContext().organization?.idand falling back touser.idkeeps the API keys flow working for both org and user subjects without invoking the new enable‑organizations dev prompt viauseOrganization. Given thewithCoreUserGuardwrapper, the user fallback is safe, and the comment makes the intent clear.Also applies to: 241-248
packages/clerk-js/bundlewatch.config.json (1)
4-7: Bundle size budget updates look reasonable for the new prompt chunkThe slight increases to the main browser bundle thresholds and the dedicated 6.5KB budget for
enableOrganizationsPrompt*.jsare consistent with introducing a small lazy‑loaded dev prompt. Nothing here looks out of line.Also applies to: 26-27
packages/clerk-js/src/ui/customizables/parseAppearance.ts (1)
30-30: LGTM! Appearance key extension follows existing patterns.The addition of
'enableOrganizationsPrompt'to theappearanceKeyunion is consistent with the existing'impersonationFab'pattern and properly integrates with the broader appearance cascade system.packages/shared/src/types/appearance.ts (2)
1038-1038: LGTM! Type alias follows established conventions.The
EnableOrganizationsThemetype alias is consistent with other theme type definitions in this file (e.g.,SignInTheme,UserProfileTheme).
1127-1130: LGTM! Appearance field properly documented.The
enableOrganizationsfield addition follows the same pattern as other component-specific appearance overrides, with clear JSDoc documentation.packages/clerk-js/src/ui/lazyModules/components.ts (2)
20-20: LGTM! Import path update reflects directory restructure.The updated path for
KeylessPromptaligns with the newdevPromptsdirectory structure, improving organization of development-mode prompt components.
30-31: LGTM! EnableOrganizationsPrompt lazy loading follows established patterns.The integration of
EnableOrganizationsPromptinto the lazy loading system is consistent with other components:
- Import path with webpack chunk name annotation
- Lazy export extracting the default module export
- Inclusion in the
ClerkComponentsregistryAlso applies to: 49-51, 153-153
packages/clerk-js/src/ui/components/devPrompts/KeylessPrompt/index.tsx (2)
8-17: LGTM! Centralized shared utilities improve maintainability.The refactoring to import common prompt primitives (
PromptContainer,basePromptElementStyles,PromptSuccessIcon,handleDashboardUrlParsing) from the shared module reduces code duplication and ensures consistency across dev prompts.
116-116: LGTM! Component replacement maintains functionality.The replacement of custom implementations with shared components (
PromptContainerandPromptSuccessIcon) maintains the same visual appearance and behavior while promoting code reuse.Also applies to: 168-173
packages/clerk-js/src/ui/lazyModules/providers.tsx (1)
212-227: LGTM! Provider follows established patterns.The
LazyEnableOrganizationsPromptProvidermirrors the structure ofLazyImpersonationFabProvider, correctly wrapping children with:
- Suspense boundary for lazy loading
- VirtualRouter for routing context
- AppearanceProvider with the correct appearance key
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (2)
76-82: LGTM! Conditional parameter inclusion prevents errors on older instances.The logic correctly builds the payload object and only includes
organization_allow_personal_accountswhen the environment supports it (i.e., whenhasPersonalAccountsEnabledis true). This prevents backend errors on older instances that don't support this parameter.
527-656: LGTM! Switch component is well-implemented with good accessibility.The custom Switch component includes:
- Proper ARIA roles (
role="switch")- Keyboard accessibility (no
tabIndex={-1})- Focus-visible styles via the parent label selector
- Controlled and uncontrolled modes
- Associated description with
aria-describedby- Reduced motion support
packages/clerk-js/src/ui/Components.tsx (5)
3-3: LGTM! Imports properly integrate the new prompt infrastructure.The imports correctly bring in:
- Type definitions (
__internal_EnableOrganizationsPromptProps)- Lazy component (
EnableOrganizationsPrompt)- Provider wrapper (
LazyEnableOrganizationsPromptProvider)Also applies to: 31-31, 45-45
76-99: LGTM! Type signatures and state properly extended.The changes correctly:
- Extend
openModalandcloseModaltype signatures to include'enableOrganizationsPrompt'- Add proper type discrimination for props based on modal type
- Initialize the state field
enableOrganizationsPromptModalAlso applies to: 101-112, 162-162, 256-256
354-367: LGTM! Deduplication logic prevents multiple prompt instances.The special handling for
enableOrganizationsPromptcorrectly prevents reopening the prompt if it's already open. This is appropriate given that multiple components/hooks on the same page might trigger the prompt, and showing multiple instances would confuse users. Based on learnings.
639-643: LGTM! Rendering follows established component patterns.The conditional rendering of
EnableOrganizationsPromptwrapped inLazyEnableOrganizationsPromptProvideris consistent with how other prompts (likeImpersonationFab) are rendered.
337-340: LGTM! Template literal property access fix is appropriate.The explicit type assertion on line 340 is necessary because TypeScript doesn't narrow types correctly when using template literal property access combined with the
inoperator. The comment clearly explains the limitation, and the cast is safe given the preceding type guard.
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
Show resolved
Hide resolved
60e0b72 to
7dfbc66
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (2)
73-93: Consider surfacing a lightweight error state when enabling failsRight now the
catchpath only clearsisLoading. From the user’s perspective the button just re‑enables with no indication whether the attempt failed or silently did nothing, which can be confusing even in a dev‑only UI.Consider adding a small error state (e.g.
const [error, setError] = useState<string | null>(null)) that you clear on start, set in thecatchblock, and render as a short inline message under the description text.Also applies to: 292-298
537-560: Add hover and focus-visible styles to Link for better affordanceThe
Linkcomponent currently renders as styled text with no hover or focus-visible feedback. For a primary navigation/action affordance (“dashboard”, “Learn more”), it’s helpful for both pointer and keyboard users to get a visible state change.You can extend the existing styles along these lines:
const Link = forwardRef<HTMLAnchorElement, React.ComponentProps<'a'> & { css?: SerializedStyles }>( ({ children, css: cssProp, ...props }, ref) => { return ( <a ref={ref} {...props} css={[ basePromptElementStyles, css` color: #a8a8ff; font-size: inherit; font-weight: 500; line-height: 1.3; font-size: 0.8125rem; min-width: 0; + cursor: pointer; + transition: color 120ms ease-in-out; + + &:hover { + color: #c4c4ff; + } + + &:focus-visible { + outline: 2px solid white; + outline-offset: 2px; + border-radius: 2px; + } + + @media (prefers-reduced-motion: reduce) { + transition: none; + } `, cssProp, ]} > {children} </a> ); }, );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (5)
packages/shared/src/types/clerk.ts (1)
__internal_EnableOrganizationsPromptProps(1449-1459)packages/clerk-js/src/ui/components/devPrompts/shared.tsx (5)
handleDashboardUrlParsing(84-100)PromptContainer(10-28)basePromptElementStyles(33-55)ClerkLogoIcon(105-176)PromptSuccessIcon(57-82)packages/shared/src/types/devtools.ts (1)
EnableEnvironmentSettingParams(3-6)packages/clerk-js/src/core/resources/DevTools.ts (1)
DevTools(8-21)packages/clerk-js/src/ui/elements/Modal.tsx (1)
Modal(26-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Static analysis
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Unit Tests (22, shared, clerk-js, RQ)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx (2)
50-53: Good handling of personal‑account support across instance versionsThe
hasPersonalAccountsEnabledguard plus conditional inclusion oforganization_allow_personal_accountsinEnableEnvironmentSettingParamsis a clean way to keep older instances from receiving unsupported flags while still wiring the toggle when the feature is present. This should address the dev_tools compatibility concern without complicating the call‑site.Also applies to: 76-82
416-463: Switch implementation looks solid from an accessibility standpointNice job on the custom switch:
- Hidden native
<input type="checkbox" role="switch">witharia-describedbyties semantics to the label/description.- Label wrapping,
:has(input:focus-visible)for focus ring, and disabled styling via:has(input:disabled)give good keyboard and pointer behavior.- Controlled/uncontrolled handling via
checked/defaultCheckedis straightforward.No changes requested here.
Also applies to: 465-531
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
Outdated
Show resolved
Hide resolved
packages/clerk-js/src/ui/components/devPrompts/EnableOrganizationsPrompt/index.tsx
Outdated
Show resolved
Hide resolved
|
I'll block release until Monday so we can deploy clerk-js safely |

Description
This PR allows developers to enable the organization's featureset in-app instead of having to go to the Clerk Dashboard, decreasing friction to build B2B apps.
The prompt only appears for development instances only.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Chores
Accessibility
✏️ Tip: You can customize this high-level summary in your review settings.